feat(query-core): add structuralSharing option to useQueries#10101
feat(query-core): add structuralSharing option to useQueries#10101dagstuan wants to merge 11 commits intoTanStack:mainfrom
Conversation
🦋 Changeset detectedLatest commit: e7a7214 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new public option Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as Framework Hook\n(useQueries)
participant Observer as QueriesObserver\n(query-core)
participant Combine as CombineFn\n(user-provided)
participant Result as Combined Result
Hook->>Observer: construct(queries, combine?, structuralSharing?)
Hook->>Observer: getOptimisticResult(queries, combine, structuralSharing)
Observer->>Observer: compute per-query optimistic results
alt combine provided
Observer->>Combine: call combine(per-query results)
Combine-->>Observer: combined value
Observer->>Observer: apply structuralSharing?
Observer-->>Result: combined (possibly shared) result
else no combine
Observer-->>Result: array of per-query results
end
Observer-->>Hook: optimistic result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/solid-query/src/useQueries.ts (1)
312-332:⚠️ Potential issue | 🔴 CriticalBug:
structuralSharingis not passed toobserver.setQueries()in the Solid adapter.Both
setQueriescall sites (lines 313–320 and 324–331) manually construct the options object with onlycombine, omittingstructuralSharing. This causes the observer's internal#options.structuralSharingto beundefinedafter mount/update, resulting in subscription-driven notifications ignoring the user'sstructuralSharing: falsesetting.All other framework adapters (React, Vue, Svelte, Angular) pass the full options object including
structuralSharingtosetQueries.Proposed fix
onMount(() => { observer.setQueries( defaultedQueries(), - queriesOptions().combine - ? ({ - combine: queriesOptions().combine, - } as QueriesObserverOptions<TCombinedResult>) - : undefined, + queriesOptions() as QueriesObserverOptions<TCombinedResult>, ) }) createComputed(() => { observer.setQueries( defaultedQueries(), - queriesOptions().combine - ? ({ - combine: queriesOptions().combine, - } as QueriesObserverOptions<TCombinedResult>) - : undefined, + queriesOptions() as QueriesObserverOptions<TCombinedResult>, ) })The extra
queriesproperty inqueriesOptions()will be passed tosetQueriesbut safely ignored at runtime sinceQueriesObserverOptionsonly hascombineandstructuralSharingproperties.
🤖 Fix all issues with AI agents
In @.changeset/silver-coins-mix.md:
- Line 10: The changeset message contains a typo "strucuralSharing"; update the
changes text to read "structuralSharing" so the feature line reads:
feat(query-core): Allow disabling structuralSharing for useQueries, ensuring the
corrected spelling replaces the incorrect "strucuralSharing" token in the
changeset entry.
In `@packages/query-core/src/queriesObserver.ts`:
- Around line 33-37: The JSDoc claim that structuralSharing disables structural
sharing is inaccurate because replaceEqualDeep is only called inside the combine
branch (see combine usage around replaceEqualDeep and return of input), so
structuralSharing: false is a no-op when combine is undefined; either update the
JSDoc (and corresponding adapter docs) to state structuralSharing only applies
when combine is provided, or change the non-combine path to respect the flag by
applying the same replaceEqualDeep logic to the returned input (use the existing
replaceEqualDeep helper and the structuralSharing boolean where the function
currently returns input directly) so structuralSharing:false actually prevents
structural sharing in both code paths (adjust code around the combine check and
the final return to call replaceEqualDeep when appropriate).
🧹 Nitpick comments (1)
packages/query-core/src/__tests__/queriesObserver.test.tsx (1)
296-305: Missing test coverage forstructuralSharing: false.All existing tests pass
undefined(defaulting totrue). There are no tests verifying thatstructuralSharing: falseactually skipsreplaceEqualDeepand produces a new reference for the combined result. Consider adding a test that asserts referential inequality whenstructuralSharingisfalsewith acombinefunction, and referential equality when it'strue.
dagstuan
force-pushed
the
main
branch
4 times, most recently
from
368a0b7 to
21e9f59
Compare
Add a `structuralSharing` option to useQueries/createQueries/injectQueries that allows disabling structural sharing for the combined result. When set to `false`, the combined result will not use `replaceEqualDeep` for referential stability. Defaults to `true`.
Match the useQuery logic and allow a function to override the structural sharing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/framework/react/reference/useQueries.md`:
- Around line 27-31: Update the `structuralSharing` option docs to state the
prerequisite that it "Only applies when `combine` is provided"; specifically, in
the `structuralSharing?: boolean | ((oldData: unknown | undefined, newData:
unknown) => unknown)` description add a concise note that this setting has
effect only if `combine` is supplied, referencing the `combine` option so
readers understand the dependency between `structuralSharing` and `combine`.
🧹 Nitpick comments (1)
packages/svelte-query/src/createQueries.svelte.ts (1)
215-216: IncludingstructuralSharinginderivedCreateQueriesOptionsmay trigger unnecessary observer recreation.Since
observeris$derivedfromderivedCreateQueriesOptions(line 229-235), changing onlystructuralSharingwill recreate the entireQueriesObserver. The constructor doesn't usestructuralSharing— it's only consumed later bygetOptimisticResultand#combineResult. This is the same pre-existing trade-off ascombinechanges causing recreation, so it's not a regression, but worth noting for potential future optimization.Also applies to: 229-235
|
@TkDodo Is there anything left for this to be reviewable again? |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 4m 24s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 2s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-02 07:58:59 UTC
| if (typeof structuralSharing === 'function') { | ||
| this.#combinedResult = structuralSharing( | ||
| this.#combinedResult, | ||
| combined, | ||
| ) as TCombinedResult | ||
| } else { | ||
| this.#combinedResult = | ||
| structuralSharing !== false | ||
| ? replaceEqualDeep(this.#combinedResult, combined) | ||
| : combined | ||
| } |
There was a problem hiding this comment.
we have a replaceData util for this that we should use here.
There was a problem hiding this comment.
I tried to fix this now, but I'm not sure if the result is satisfactory. The replaceData util required QueryOptions<any, any, any, any> which didn't map cleanly to my usage inside of queriesObserver.. You can take a look and see how you like it, I had to change the signature slightly.
| /** | ||
| * Set this to `false` to disable structural sharing between query results. | ||
| * Set this to a function which accepts the old and new data and returns resolved data of the same type to implement custom structural sharing logic. | ||
| * Only applies when `combine` is provided. |
There was a problem hiding this comment.
I’m wondering if it should also apply to normal useQueries results without combine?
There was a problem hiding this comment.
How would that work? Currently all of the logic related to structural sharing and combining of the results in useQueries rely on a combine function to be specified. If the consumer wants custom structural sharing without a comine they can just use the regular structuralSharing option on each query sent into useQueries instead?
|
I attempted to fix your comments now @TkDodo 🙏 Hope this can get merged eventually 😅 |
2 participants
🎯 Changes
Add a
structuralSharingoption to useQueries/createQueries/injectQueries that allows disabling structural sharing for the combined result. When set tofalse, the combined result will not usereplaceEqualDeepfor referential stability. Defaults totrue.Full disclosure: Docs were generated by Claude, all other implementation was done without AI.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Chores